-
Notifications
You must be signed in to change notification settings - Fork 26
Add nightly dependency bump workflow #479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add nightly dependency bump workflow #479
Conversation
I just realized: I updated main build so that it also triggers when pushes are made to the |
This reverts commit 076e529.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a complete review, just some initial thoughts.
with: | ||
ref: ${{ inputs.ref || github.sha }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why github.sha
? Is there no github.ref
? Just a bit unclear why this is correct.
.github/workflows/main-build.yml
Outdated
needs: [ build, application-signals-e2e-test ] | ||
runs-on: ubuntu-latest | ||
if: always() | ||
if: always() && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release/')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need always()
? Also release/v
I think.
if: always() && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release/')) | |
if: (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release/v')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did we confirm that refs/heads/main
is correct?
@@ -0,0 +1,147 @@ | |||
name: Nightly Upstream Snapshot Build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets add a description
.github/workflows/nightly-build.yml
Outdated
- name: Set up Python | ||
uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c #v6.0.0 | ||
with: | ||
python-version: '3.11' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 3.11?
OTEL_PYTHON_VERSION: ${{ steps.get_versions.outputs.otel_python_version }} | ||
OTEL_CONTRIB_VERSION: ${{ steps.get_versions.outputs.otel_contrib_version }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, does that work? I didn't realize a script could send outputs to the GH action like this. Just want to confirm we tested this.
|
||
# Check if this release is between current and new version | ||
try: | ||
if version.parse(release_version) > version.parse(current_version) and version.parse( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sanity check: Does this handle patch versions well? Would like to see a bit of testing here, can just be a manual thing.
if any( | ||
keyword in body for keyword in ["breaking change", "breaking changes", "breaking:", "breaking"] | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this based on?
Does this work for [BREAKING]
, which I saw here?
} | ||
) | ||
except (ValueError, KeyError): | ||
# Skip releases with invalid version formats or missing data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets at least log it.
except requests.RequestException as request_error: | ||
print(f"Warning: Could not get releases for {repo}: {request_error}") | ||
return [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we fail?
**Upstream releases with breaking changes:** | ||
${{ steps.breaking_changes.outputs.breaking_changes_info }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Elaborate just a bit - callout this is not a perfect system and say what we are looking for.
Issue #, if available:
Description of changes:
Add a workflow
nightly-build.yml
with following actions:nightly-dependency-updates
branchExample nightly build PR: #484
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.